-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
src/withdrawal: Allow deletion of withdrawal records #582
src/withdrawal: Allow deletion of withdrawal records #582
Conversation
When withdrawal record is deleted decrement blockedAmount of the vault, with the amount of the withdrawal
✅ Tests will run for this PR. Once they succeed it can be merged. |
In case withdrawal record with status === succeeded has been deleted, vault's amount will be incremented by the withdrawal amount. For any other status blockedAmount field will be decremented
where: { id: withdrawal.sourceVaultId }, | ||
data: { | ||
amount: { | ||
increment: withdrawal.status === WithdrawStatus.succeeded ? withdrawal.amount : 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we have this status condition in the unit test?
Does this fluctuate?
.catch(() => { | ||
throw new BadRequestException("Withdrawal record couldn't be deleted") | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to check the status before updating, instead of incrementing with 0...
Don't decrement blockedAmount if withdrawal status is cancelled or declined
Added test cases for deletion of withdrawal records with status cancelled,declined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand now...
When withdrawal record is deleted decrement blockedAmount of the vault, with the amount of the withdrawal
Closes: Reported in discord